fix!: options in .editorconfig not work with shfmt #1165

adoyle-h · 2024-05-15T15:28:24Z

Must pass --filename option to shfmt.

When shfmt read content from stdin, it doesn't know the filename and its extension. So it won't match the patterns "[.sh]" and "[.bash]" in .editorconfig.

For example, my .editorconfig file in project root.

[*.{bash,sh}]
indent_style = space
indent_size = 2
charset = utf-8

### Below are shfmt options ###
# --language-variant
shell_variant      = bash
binary_next_line   = false
# --case-indent
switch_case_indent = true
space_redirects    = true
keep_padding       = false
# --func-next-line
function_next_line = false

You can compare the results between cat ./your-script.sh | shfmt --filename=your-script.sh -d - and cat ./your-script.sh | shfmt -d -.

Do not pass any Parser and Printer options like -i/-p/-bn/-l. It will cause the .editorconfig not to be loaded. See https://github.com/mvdan/sh/blob/23633a432f903599a4ce46c30c4337e413a26ef1/cmd/shfmt/main.go#L186-L196

Breaking Change:

Removed shfmtConfig options. Use the .editorconfig options instead of. The .editorconfig options of shfmt refer to https://github.com/mvdan/sh/blob/master/cmd/shfmt/shfmt.1.scd#examples

adoyle-h · 2024-05-15T15:36:19Z

I recommend to use the .editorconfig to set the shfmt instead of global config in nvim. It is simple and has many benefits.

Related issue: #1161

Must pass --filename option to shfmt. When shfmt read content from stdin, it doesn't know the filename and its extension. So it won't match the patterns "[*.sh]" and "[*.bash]" in .editorconfig. Do not pass any Parser and Printer options like -i/-p/-bn/-l. It will cause the .editorconfig not to be loaded. See https://github.com/mvdan/sh/blob/23633a432f903599a4ce46c30c4337e413a26ef1/cmd/shfmt/main.go#L186-L196 Breaking Change: Removed shfmtConfig options. Use the .editorconfig options instead of. The .editorconfig options of shfmt refer to https://github.com/mvdan/sh/blob/master/cmd/shfmt/shfmt.1.scd#examples

chris-reeves · 2024-05-24T15:11:50Z

This change would require users to create an .editorconfig in order to configure shfmt. I think a better approach would be to use .editorconfig to set the formatting options if present, whilst still supporting configuration via the editor. It already does this for indentation if your editor is set up correctly, but will need special handling for the other options. I did consider this as part of the original implementation, but didn't want to overcomplicate things too early! If there's demand then I'm happy to look into this.

adoyle-h · 2024-05-25T07:38:31Z

The simplest approach is to create a ~/.editorconfig for global configuration for editor, and .editorconfig under project root for project specifics.
Current options of shfmtConfig are incomplete that lacking of "keep_padding", "shell_variant", "indent_style" and "indent_size". And when shfmt changed its options, bash-language-server has to change and release new version. That is overcomplicate.

And the .editorconfig file can be store in git to share with co-workers. But options in editor configuration like vim codes are unshareable.

This wasn't included initially as it has been deprecated: mvdan/sh#658 However, it has been mentioned in bash-lsp#1165 and is a valid option in the current version of `shfmt`, so support has been added (with a suitable deprecation warning).

chris-reeves · 2024-06-01T23:48:42Z

The simplest approach is to create a ~/.editorconfig for global configuration for editor, and .editorconfig under project root for project specifics.

That's fine as long as people are happy doing that, but not all users will want to - some will want to configure this via their editor as they do for other options.

Current options of shfmtConfig are incomplete that lacking of "keep_padding", "shell_variant", "indent_style" and "indent_size".

keep_padding was left out as it was deprecated, but my latest PR adds this in as it has been mentioned more than once.
shell_variant is still not supported via editor configuration as shfmt's detection is pretty good. Again, if there's demand then it can be added.
Indentation configuration is not specific to shfmt, so you configure this as usual in your editor (or via .editorconfig) and shfmt will use that. My latest PR clarifies this in the README.

And when shfmt changed its options, bash-language-server has to change and release new version. That is overcomplicate.

Fair point. But they don't change frequently so I don't think that's a good reason not to give users the option to configure this via their editor.

And the .editorconfig file can be store in git to share with co-workers. But options in editor configuration like vim codes are unshareable.

Yes, I understand the benefits of .editorconfig and use this myself for some projects, but not everyone wants to do that. I believe in giving users a choice, so think both ways should be supported. I'll raise another PR soon enough to add support for .editorconfig in addition to editor config. :-p

Shane-XB-Qian · 2024-06-02T11:29:29Z

> And the .editorconfig file can be store in git to share with co-workers. But options in editor configuration like vim codes are unshareable. Yes, I understand the benefits of `.editorconfig` and use this myself for some projects, but not everyone wants to do that. I believe in giving users a choice, so think both ways should be supported. I'll raise another PR soon enough to add support for `.editorconfig` in addition to editor config. :-p

Try 'modeline' of vim, or simple project local '.foo_vimrc' to share, if you like. // not everyone was a fan of editorconfig such thing.

…

-- shane.xb.qian

adoyle-h · 2024-06-02T15:25:55Z

@chris-reeves Thanks for the detailed explanation. I agree with giving users more choices.

@Shane-XB-Qian How to set these options of shfmt in vim modeline?
The project specific vimrc seems to be a good idea. But it only works for oneself but not co-workers. Because the project specific vimrc may have conflicts with co-workers' global vimrc.

Shane-XB-Qian · 2024-06-02T21:36:59Z

those just some cfg about indent etc, event no lsp, vim can use shfmt as bash formater, not some fancy.

…

-- shane.xb.qian

adoyle-h · 2024-06-04T18:05:43Z

The shfmt options in .editorconfig still not work on version 5.3.4. The key point at:

Do not pass any Parser and Printer options like -i/-p/-bn/-l. It will cause the .editorconfig not to be loaded. See mvdan/sh@23633a4/cmd/shfmt/main.go#L186-L196

If we decide to support shfmt options both in .editorconfig and in editor config, it will get complicated.

It is necessary to look up .editorconfig file recursively, and parse the .editorconfig content in bash-lsp. Then the merged the options from editorconfig and shfmtConfig would be passed to shfmt.

The shfmt options in .editorconfig must have higher weight than it in shfmtConfig. Otherwise, the options in .editorconfig would never work because the shfmtConfig always override them.

If only check the .editorconfig file existed and don't parse and merge the .editorconfig options, it is not enough. User may write a .editorconfig for markdown/lua/js files but not for shellscripts.
The bash-lsp must know the shfmt options in .editorconfig for current file to decide whether use shfmtConfig as default or not.

I think it is a bad design that parsing .editorconfig twice in both bash-lsp and shfmt.
What's your advice? @chris-reeves

chris-reeves · 2024-06-04T23:41:33Z

You're right that we're going to have to be a wee bit clever about this. As you say, just looking for an .editorconfig in the path isn't going to cut it as it may not apply to shell scripts. Likewise we can't blindly apply .editorconfig if there is shell script config in there, because it might only specify indentation config with no shfmt options. My plan is to read and parse the .editorconfig config for the file, combine it with the existing language server config (with a configurable priority) and then pass those options to shfmt. So .editorconfig will only be parsed once (by the language server, not shfmt).

It sounds quite complicated, but should be fairly straightforward to implement. I have this sketched out in my head and should have something by the weekend, assuming I get some time on this over the next couple of evenings.

chris-reeves · 2024-06-10T07:48:46Z

@adoyle-h #1171 has been merged. If that meets your needs/expectations then I think you can close this PR.

adoyle-h · 2024-06-11T02:34:12Z

@chris-reeves It looks good to me. Thanks.

adoyle-h force-pushed the fix/shfmt-config branch from 15729f3 to ea99523 Compare May 15, 2024 15:32

adoyle-h force-pushed the fix/shfmt-config branch from ea99523 to 86e7671 Compare May 15, 2024 16:49

chris-reeves mentioned this pull request Jun 1, 2024

Additional shfmt options #1168

Merged

chris-reeves mentioned this pull request Jun 7, 2024

Add .editorconfig support for shfmt #1171

Merged

adoyle-h closed this Jun 11, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix!: options in .editorconfig not work with shfmt #1165

fix!: options in .editorconfig not work with shfmt #1165

Uh oh!

adoyle-h commented May 15, 2024 •

edited

Loading

Uh oh!

adoyle-h commented May 15, 2024

Uh oh!

chris-reeves commented May 24, 2024

Uh oh!

adoyle-h commented May 25, 2024 •

edited

Loading

Uh oh!

chris-reeves commented Jun 1, 2024 •

edited

Loading

Uh oh!

Shane-XB-Qian commented Jun 2, 2024 via email

Uh oh!

adoyle-h commented Jun 2, 2024

Uh oh!

Shane-XB-Qian commented Jun 2, 2024 via email

Uh oh!

adoyle-h commented Jun 4, 2024

Uh oh!

chris-reeves commented Jun 4, 2024

Uh oh!

chris-reeves commented Jun 10, 2024

Uh oh!

adoyle-h commented Jun 11, 2024

Uh oh!

Uh oh!

fix!: options in .editorconfig not work with shfmt #1165

fix!: options in .editorconfig not work with shfmt #1165

Uh oh!

Conversation

adoyle-h commented May 15, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

adoyle-h commented May 15, 2024

Uh oh!

chris-reeves commented May 24, 2024

Uh oh!

adoyle-h commented May 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chris-reeves commented Jun 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Shane-XB-Qian commented Jun 2, 2024 via email

Uh oh!

adoyle-h commented Jun 2, 2024

Uh oh!

Shane-XB-Qian commented Jun 2, 2024 via email

Uh oh!

adoyle-h commented Jun 4, 2024

Uh oh!

chris-reeves commented Jun 4, 2024

Uh oh!

chris-reeves commented Jun 10, 2024

Uh oh!

adoyle-h commented Jun 11, 2024

Uh oh!

Uh oh!

adoyle-h commented May 15, 2024 •

edited

Loading

adoyle-h commented May 25, 2024 •

edited

Loading

chris-reeves commented Jun 1, 2024 •

edited

Loading